{
  "verdict": "request-changes",
  "summary": "The enum expansion is correct and backward-compatible at the validation layer, but GoalFrontmatter.Status() lacks normalization — unlike TaskFrontmatter.Status() — creating an asymmetric read path where legacy goal statuses don't canonicalize to task-aligned values. Additionally, the new goal status enum values have no dedicated test coverage.",
  "comments": [
    {
      "file": "pkg/domain/goal_frontmatter.go",
      "line": 29,
      "severity": "major",
      "message": "Should Fix (Important): GoalFrontmatter.Status() returns raw values without normalization, unlike TaskFrontmatter.Status() which applies NormalizeTaskStatus. A goal file with \"status: active\" returns GoalStatusActive rather than GoalStatusInProgress, breaking semantic alignment with TaskStatus at the read layer. Add NormalizeGoalStatus mirroring NormalizeTaskStatus and apply it in Status()."
    },
    {
      "file": "pkg/domain/",
      "line": 0,
      "severity": "major",
      "message": "Should Fix (Important): No test file for goal status enum. task_status_test.go covers all canonical values, normalization, and aliases; goal_status_test.go does not exist. New values (GoalStatusNext, GoalStatusInProgress, GoalStatusBacklog, GoalStatusHold, GoalStatusAborted) and legacy backward-compatibility paths lack any unit tests."
    },
    {
      "file": "pkg/domain/goal.go",
      "line": 63,
      "severity": "nit",
      "message": "Nice to Have (Optional): Comment says 'all valid canonical goal status values' but AvailableGoalStatuses includes legacy non-canonical constants GoalStatusActive and GoalStatusOnHold. Clarify the comment or split into canonical/legacy slices."
    }
  ],
  "concerns_addressed": [
    "correctness: AvailableGoalStatuses validates both canonical and legacy values — validation logic is correct, legacy values pass validation. However, the normalization layer (NormalizeGoalStatus) is missing, creating asymmetry with how TaskStatus handles this.",
    "correctness: Backward compatibility for existing goal files is preserved — files with legacy status values deserialize and validate correctly. But the lack of normalization means GoalStatusActive is a distinct value from GoalStatusInProgress at the type level, not a true alias.",
    "tests: No test coverage for goal status enum — confirmed. goal_status_test.go does not exist, and no tests exercise the new canonical values."
  ]
}

{
  "verdict": "approve",
  "summary": "The goal status enum expansion is well-designed and correctly adds the 6 canonical statuses aligned with TaskStatus, preserving backward compatibility with legacy values. The two concerns from the PR plan are either not bugs (duplicate validation entries) or are by-design behavior (legacy values tolerated but not normalized). One should-fix remains: no automated test coverage for the new GoalStatus values.",
  "comments": [
    {
      "file": "pkg/domain/goal.go",
      "line": 86,
      "severity": "major",
      "message": "Should Fix: GoalStatus.Validate() accepts legacy aliases (active, on_hold) but never normalizes them to canonical forms (in_progress, hold). Compare with TaskStatus.NormalizeTaskStatus() which converts legacy→canonical at write time. If behavioral alignment with TaskStatus is the goal, consider adding a NormalizeGoalStatus() function so legacy goal statuses get migrated to canonical values when written."
    },
    {
      "file": "pkg/domain/goal.go",
      "line": 1,
      "severity": "nit",
      "message": "Nice to Have: No dedicated goal_status_test.go exists. The new canonical statuses (next, in_progress, backlog, hold, aborted) and legacy aliases (active, on_hold) lack automated validation tests. Consider adding tests mirroring the pattern in task_status_test.go."
    }
  ],
  "concerns_addressed": [
    "correctness: semantic aliases are string-different but tolerated (not normalized) — Validate() accepts legacy values but does not convert them. This is by design but falls short of full TaskStatus alignment.",
    "correctness: AvailableGoalStatuses expansion does not produce duplicate validation entries — Contains() is a simple linear scan over a flat list, no issue.",
    "tests: no automated test coverage for new status values — manual test vault-cli goal set ... status in_progress confirmed needed by PR body. Automated tests are missing."
  ]
}

{
  "verdict": "request-changes",
  "summary": "The goal status enum expansion is well-designed and backward-compatible, but the version alignment check fails: CHANGELOG.md has been reverted to `## Unreleased` while the three plugin JSON files (`.claude-plugin/plugin.json`, `.claude-plugin/marketplace.json`) still carry `v0.89.0`. All four version strings must match per CLAUDE.md § Version Alignment. The code itself is correct.",
  "comments": [
    {
      "file": ".claude-plugin/plugin.json",
      "line": 1,
      "severity": "major",
      "message": "Should Fix: version field is `\"0.89.0\"` but the feature is a minor release (new goal statuses aligned with task statuses). Per CLAUDE.md § Version Alignment, all four version strings must equal each other. Update to `\"0.90.0\"` to match the changelog entry in CHANGELOG.md."
    },
    {
      "file": ".claude-plugin/marketplace.json",
      "line": 1,
      "severity": "major",
      "message": "Should Fix: both `metadata.version` and `plugins[0].version` fields are `\"0.89.0\"`. Per CLAUDE.md § Version Alignment, all four version strings must equal each other. Update both to `\"0.90.0\"` to match CHANGELOG.md."
    }
  ],
  "concerns_addressed": [
    "correctness: GoalStatusActive kept as legacy alias — Verified: AvailableGoalStatuses includes all 8 values (6 canonical + 2 legacy), SetStatus validates against AvailableGoalStatuses, so callers using string comparison 'active' still work correctly",
    "correctness: Semantic aliasing (active ↔ in_progress, on_hold ↔ hold) documented in comments — Confirmed: pkg/domain/goal.go:38-61 documents this clearly. No business logic beyond validation depends on these equivalences"
  ]
}